Skip to content

Consolidate dialect duplication and make mypy strict pass cleanly#23

Merged
richardwooding merged 1 commit into
mainfrom
refactor/dialect-dedup-and-mypy-clean
Jun 14, 2026
Merged

Consolidate dialect duplication and make mypy strict pass cleanly#23
richardwooding merged 1 commit into
mainfrom
refactor/dialect-dedup-and-mypy-clean

Conversation

@richardwooding

Copy link
Copy Markdown
Contributor

Summary

Two behavior-preserving maintainability passes. The 728 unit tests are unchanged and pass, generated SQL is byte-identical, and mypy --strict now reports 0 errors (was 171).

1. Dialect de-duplication

The six dialect files repeated many byte-identical write_* methods, so any change had to be hand-applied to every copy.

  • Moved clear-majority shared methods to the Dialect base class as concrete defaults: write_string_literal, write_string_concat (||), write_duration, write_interval, write_timestamp_arithmetic, write_struct_close, and write_type_name (via a TYPE_MAP class attribute). Dialects override only where they genuinely diverge (e.g. BigQuery string escaping, MySQL/Spark CONCAT, SQLite interval modifiers).
  • Added validate_sql_field_name() in _utils shared by DuckDB/BigQuery/MySQL/SQLite (parameterized by reserved set + length + dialect label).
  • Refactored convert_re2_to_posix to reuse _validate_regex_common (dropped ~60 duplicated validation lines).
  • Extracted field_is_json() (in schema.py), shared by _converter and _analysis.

Net duplicate-function clusters dropped 20 → 15 (remaining ones are 2–3-dialect minority overlaps, left explicit by design).

2. mypy strict clean (171 → 0)

All 171 errors were typing gaps, not runtime bugs.

  • Parametrized lark generics: Tree[Token] (via a TreeT alias), list[...], Interpreter[Token, None].
  • Modeled child nodes as Branch = Tree[Token] | Token to match lark's Tree.children, widening child-consuming helpers and adding a small _children() helper. isinstance(..., Tree) runtime checks were left untouched (parametrized generics can't be used in isinstance).
  • Fixed six one-offs: replaced two tuple-expression lambdas with named def ... -> None helpers, renamed a float-branch variable, added a missing return annotation.
  • Dropped the "pre-existing lark type errors are expected" note from CLAUDE.md.

Test plan

  • uv run pytest tests/ --ignore=tests/integration → 728 passed
  • uv run mypy src/pycel2sql/ → Success, no issues (was 171 errors)
  • uv run ruff check src/ tests/ → clean
  • Integration tests (tests/integration/) deferred to CI — no local container runtime available.

🤖 Generated with Claude Code

Two behavior-preserving maintainability passes (728 unit tests unchanged,
generated SQL byte-identical):

Dialect de-duplication:
- Move clear-majority shared methods to the Dialect base class as concrete
  defaults (write_string_literal, write_string_concat, write_duration,
  write_interval, write_timestamp_arithmetic, write_struct_close,
  write_type_name via a TYPE_MAP class attr). Dialects override only where
  they diverge.
- Add validate_sql_field_name() in _utils and share it across DuckDB/
  BigQuery/MySQL/SQLite; refactor convert_re2_to_posix to reuse
  _validate_regex_common; extract field_is_json() shared by _converter and
  _analysis.

mypy strict clean (171 -> 0 errors):
- Parametrize lark generics: Tree[Token] (via a TreeT alias), list[...],
  Interpreter[Token, None]. Model child nodes as Branch = Tree[Token] | Token
  to match lark's Tree.children, widening child-consuming helpers and adding
  a _children() helper. isinstance(..., Tree) runtime checks left untouched.
- Fix six one-offs: replace two tuple-expression lambdas with named helpers,
  rename a float-branch variable, add a missing return annotation.
- Drop the "pre-existing lark type errors are expected" note from CLAUDE.md.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@richardwooding richardwooding merged commit c582232 into main Jun 14, 2026
7 checks passed
@richardwooding richardwooding deleted the refactor/dialect-dedup-and-mypy-clean branch June 14, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant